Add command result support for resource commands#15622
Add command result support for resource commands#15622
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 15622Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15622" |
|
Something I've been playing around with in my JWT scenario is writing the output directly to the user's clipboard - something worth thinking about. Works reasonably well for secrets |
davidfowl
force-pushed
the
davidfowl/command-results
branch
2 times, most recently
from
2f9c523 to
fc149ed
Compare
davidfowl
requested review from
JamesNK,
adamint,
danegsta,
karolz-ms and
mitchdenny
as code owners
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for resource commands to return structured result data (text/JSON) back to callers (Dashboard, CLI, MCP), flowing from hosting model → backchannel/gRPC → UI/CLI.
Changes:
- Introduces
CommandResultFormatand extendsExecuteCommandResult/CommandResults.Success(...)to carry result payload + format. - Extends dashboard gRPC/backchannel contracts and mapping to transport
result/result_format. - Updates Dashboard/CLI/MCP to surface command results, plus adds/updates tests and regenerated polyglot codegen snapshots.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.Tests/ResourceCommandServiceTests.cs | Adds hosting tests for result propagation and replica aggregation behavior. |
| tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.ts | Updates TS generated API snapshots for new fields/enum. |
| tests/Aspire.Hosting.CodeGeneration.Rust.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.rs | Updates Rust generated API snapshots for new fields/enum. |
| tests/Aspire.Hosting.CodeGeneration.Python.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.py | Updates Python generated API snapshots for new fields/enum. |
| tests/Aspire.Hosting.CodeGeneration.Java.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.java | Updates Java generated API snapshots for new fields/enum. |
| tests/Aspire.Hosting.CodeGeneration.Go.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.go | Updates Go generated API snapshots for new fields/enum. |
| tests/Aspire.Cli.Tests/TestServices/TestInteractionService.cs | Adds a hook to capture DisplayMessage calls in CLI tests. |
| tests/Aspire.Cli.Tests/Mcp/ExecuteResourceCommandToolTests.cs | Verifies MCP tool returns additional content block when a command returns result data. |
| tests/Aspire.Cli.Tests/Commands/ResourceCommandHelperTests.cs | Adds CLI tests around displaying command results. |
| src/Aspire.Hosting/Dashboard/proto/dashboard_service.proto | Adds result and result_format fields + CommandResultFormat enum to the dashboard proto. |
| src/Aspire.Hosting/Dashboard/DashboardServiceData.cs | Extends command execution tuple to include result payload + format. |
| src/Aspire.Hosting/Dashboard/DashboardService.cs | Maps hosting command results into gRPC response fields. |
| src/Aspire.Hosting/Backchannel/BackchannelDataTypes.cs | Adds backchannel DTO properties for result payload + format. |
| src/Aspire.Hosting/Backchannel/AuxiliaryBackchannelRpcTarget.cs | Maps hosting ExecuteCommandResult to backchannel result fields. |
| src/Aspire.Hosting/ApplicationModel/ResourceCommandService.cs | Aggregates replica results and returns the first successful result payload/format. |
| src/Aspire.Hosting/ApplicationModel/ResourceCommandAnnotation.cs | Adds CommandResultFormat, ExecuteCommandResult.Result/ResultFormat, and CommandResults.Success(...) overload. |
| src/Aspire.Dashboard/ServiceClient/Partials.cs | Maps gRPC result fields into dashboard view model. |
| src/Aspire.Dashboard/Model/ResourceCommandResponseViewModel.cs | Adds result payload + format to the dashboard model and defines CommandResultFormat. |
| src/Aspire.Dashboard/Model/DashboardCommandExecutor.cs | Opens a text visualizer dialog on successful command results (locks to JSON when applicable). |
| src/Aspire.Cli/Mcp/Tools/ExecuteResourceCommandTool.cs | Returns command result as an additional MCP TextContentBlock on success. |
| src/Aspire.Cli/Commands/ResourceCommandHelper.cs | Displays command result in CLI after successful execution. |
| playground/Stress/Stress.AppHost/Program.cs | Adds sample commands returning JSON/text results in the stress playground. |
davidfowl
force-pushed
the
davidfowl/command-results
branch
from
6c96bb0 to
4619b34
Compare
davidfowl
force-pushed
the
davidfowl/command-results
branch
from
4619b34 to
0898d6d
Compare
davidfowl
force-pushed
the
davidfowl/command-results
branch
from
7c18477 to
968ae8f
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
davidfowl
force-pushed
the
davidfowl/command-results
branch
from
968ae8f to
5f7901c
Compare
|
Re-running the failed jobs in the CI workflow for this pull request because 1 job was identified as retry-safe transient failures in the CI run attempt.
|
| { | ||
| return new ExecuteCommandResult { Success = true }; | ||
| var successWithResult = results.FirstOrDefault(r => r.Result is not null); | ||
| return new ExecuteCommandResult |
There was a problem hiding this comment.
What happens if there are different replicas of a resource? Should the result aggregate all results and send those back in an array of results or something like that?
There was a problem hiding this comment.
re @joperezr's replicas comment, yes, it should - maybe also have the ability to run a command on only one instance?
|
🎬 CLI E2E Test Recordings — 52 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #23660492243 |
| message ResourceCommandResponse { | ||
| ResourceCommandResponseKind kind = 1; | ||
| optional string error_message = 2; | ||
| optional string result = 3; |
There was a problem hiding this comment.
What happens when apphost and dashboard are different versions? Like for instance, with ACA or standalone dashboard? Should we be revving the api version?
| logger.LogDebug("Executing command '{CommandName}' on resource '{ResourceName}'", commandName, resourceName); | ||
|
|
||
| // Route status messages to stderr so command results can be piped (e.g., | jq) | ||
| interactionService.Console = ConsoleOutput.Error; |
There was a problem hiding this comment.
this doesn't seem right, shouldn't we be adding a parameter to ShowStatusAsync that accepts ConsoleOutput, like DisplayRawText does?
There was a problem hiding this comment.
No we want all messages to go to stderr and output to be stdout. This is similar to what we do when —format json is specified
| /// <summary> | ||
| /// No result data. | ||
| /// </summary> | ||
| None = 0, |
There was a problem hiding this comment.
why is this necessary if result is nulalble? resultformat could be nullable as well? or we could encapsulate result and resultformat in a wrapper type, like CommandResult
| /// <summary> | ||
| /// No result data. | ||
| /// </summary> | ||
| None = 0, |
| { | ||
| return new ExecuteCommandResult { Success = true }; | ||
| var successWithResult = results.FirstOrDefault(r => r.Result is not null); | ||
| return new ExecuteCommandResult |
There was a problem hiding this comment.
re @joperezr's replicas comment, yes, it should - maybe also have the ability to run a command on only one instance?
| /// <summary> | ||
| /// Gets the result data produced by the command. | ||
| /// </summary> | ||
| public string? Result { get; init; } |
| ResourceCommandResponseKind kind = 1; | ||
| optional string error_message = 2; | ||
| optional string result = 3; | ||
| CommandResultFormat result_format = 4; |
There was a problem hiding this comment.
shouldn't this be optional too? it's elsewhere nullable
| } | ||
|
|
||
| enum CommandResultFormat { | ||
| COMMAND_RESULT_FORMAT_NONE = 0; |
There was a problem hiding this comment.
as mentioned above, having the none type feels strange
| /// </summary> | ||
| /// <param name="result">The result data.</param> | ||
| /// <param name="resultFormat">The format of the result data. Defaults to <see cref="CommandResultFormat.Text"/>.</param> | ||
| public static ExecuteCommandResult Success(string result, CommandResultFormat resultFormat = CommandResultFormat.Text) => new() { Success = true, Result = result, ResultFormat = resultFormat }; |
There was a problem hiding this comment.
Should result be nullable here, to match that we allow null results in the model?
| /// <param name="errorMessage">The error message.</param> | ||
| /// <param name="result">The result data.</param> | ||
| /// <param name="resultFormat">The format of the result data. Defaults to <see cref="CommandResultFormat.Text"/>.</param> | ||
| public static ExecuteCommandResult Failure(string errorMessage, string result, CommandResultFormat resultFormat = CommandResultFormat.Text) => new() { Success = false, ErrorMessage = errorMessage, Result = result, ResultFormat = resultFormat }; |
There was a problem hiding this comment.
Same here, should errorMessage and result be nullable?
| { | ||
| Kind = responseKind, | ||
| ErrorMessage = errorMessage ?? string.Empty | ||
| ErrorMessage = errorMessage ?? string.Empty, |
There was a problem hiding this comment.
Should this also use null to represent "no error message"?
There was a problem hiding this comment.
I think this is a grpc quirk
| Kind = responseKind, | ||
| ErrorMessage = errorMessage ?? string.Empty | ||
| ErrorMessage = errorMessage ?? string.Empty, | ||
| ResultFormat = (Aspire.DashboardService.Proto.V1.CommandResultFormat)resultFormat |
There was a problem hiding this comment.
Will the various types for CommandResultFormat always change in sync? Using an explicit switch to map the values would help catching if we miss doing that. Here and the other places where we cast for similar types.
| { | ||
| logger.LogDebug("Executing command '{CommandName}' on resource '{ResourceName}'", commandName, resourceName); | ||
|
|
||
| // Route status messages to stderr so command results can be piped (e.g., | jq) |
There was a problem hiding this comment.
| // Route status messages to stderr so command results can be piped (e.g., | jq) | |
| // Route status messages to stderr so command results in stdout remain pipeable (e.g., | jq) |
| // Route status messages to stderr so command results can be piped (e.g., | jq) | ||
| interactionService.Console = ConsoleOutput.Error; |
There was a problem hiding this comment.
Can we add a focused test for the new stream-routing behavior here?
Description
Adds
ResultandResultFormatproperties toExecuteCommandResult, allowing resource commands to return structured output data (text or JSON) back to the caller. The result flows through the entire pipeline: model → proto/gRPC → backchannel → Dashboard UI → CLI → MCP tools.Model: New
CommandResultFormatenum (None,Text,Json),Result/ResultFormatonExecuteCommandResult, andCommandResults.Success(result, format)overload.Proto/gRPC:
ResourceCommandResponsegetsoptional string resultandCommandResultFormat result_formatfields.Backchannel:
ExecuteResourceCommandResponsegainsResultandResultFormat(string) properties.Dashboard: On command success with result data, opens
TextVisualizerDialogwith syntax highlighting. JSON results lock the viewer to JSON mode.CLI: Result written to stdout via
DisplayRawText(result, ConsoleOutput.Standard). Status messages routed to stderr, enabling clean piping (e.g.,aspire resource myResource generate-token | jq .token). MCP tool returns result as additionalTextContentBlock.Replica aggregation: Takes the first successful replica's result when multiple replicas return data.
Playground: Added "Generate Token" (JSON) and "Get Connection String" (text) demo commands on
stress-telemetryservice.Fixes #15610
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: